-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mdns]: Fixes clang-tidy warnings #732
base: master
Are you sure you want to change the base?
[mdns]: Fixes clang-tidy warnings #732
Conversation
ddb4b36
to
e37b8a6
Compare
@zwx1995esp PTAL, too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question, LGTM overall.
components/mdns/mdns.c
Outdated
@@ -5346,7 +5351,7 @@ static void _mdns_service_task(void *pvParameters) | |||
for (;;) { | |||
if (_mdns_server && _mdns_server->action_queue) { | |||
if (xQueueReceive(_mdns_server->action_queue, &a, portMAX_DELAY) == pdTRUE) { | |||
if (a && a->type == ACTION_TASK_STOP) { | |||
if (a == NULL || a->type == ACTION_TASK_STOP) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: does a == NULL
mean that an exception occurs? If yes, at this time, should we stop mdns timer? Should we free the lock and deinit the queue? How about asserting right now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's definitely an invalid state, which should never happen -- i.e. typically happens during mem corrupt issues.
assert()
is probably a better reaction then just stopping the task.
Have fixed, thanks for the suggestion!
We did check for null-deref before checking 'a->type', but contol continues and passes potential null-ptr to the processing function _mdns_execute_action() Fixed by asserting action != NULL, since it is an invalid state which should never happen.
Since it was flagged by clang-tidy as insecture API
Fixing: warning: Value stored to 'shared' is never read [clang-analyzer-deadcode.DeadStores]
and some minor leaks in creation of browse results
e37b8a6
to
196198e
Compare
Fixes: